Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Menu: auto-generate README #68249

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from
Open

Menu: auto-generate README #68249

wants to merge 5 commits into from

Conversation

ciampo
Copy link
Contributor

@ciampo ciampo commented Dec 23, 2024

What?

Convert the Menu README to an auto-generated one.

Supplementary information that was in the existing README is moved to other appropriate locations (JSDocs, new "Best Practices" Storybook page).

Why?

To decrease maintenance cost and consolidate the canonical docs for our audience.

Testing Instructions

  • In your IDE, check imports of Menu.Item for example, and see that the JSDoc includes the subcomponent description.
  • npm run docs:components to regenerate READMEs.
  • See Storybook docs for Menu.

@ciampo ciampo force-pushed the feat/menu-generate-readme branch 2 times, most recently from 93215f7 to 323f875 Compare December 23, 2024 16:14

const meta: Meta< typeof Menu > = {
id: 'components-experimental-menu',
id: 'components-menu',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was necessary because the generated README links to ?path=/docs/components-menu--docs. I've updated storybook/manager-head.html to add a redirect from the old experimental URL.

* It can optionally contain one instance of the `Menu.ItemLabel` component
* and one instance of the `Menu.ItemHelpText` component.
*/
Item: Object.assign( Item, {
Copy link
Contributor Author

@ciampo ciampo Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the previous subcomponent named export scheme (ie. MenuItem being assigned to the Item property of the Menu object), the README generator could not pick up the JSDoc correctly,

I managed to fix it by changing the internal naming convention of all subcomponent by removing the Menu prefix: for example, MenuItem got renamed to Item. A lot of the code changes in this file are related to this refactor and were applied as a single commit. You can review the rest of the changes commit by commit to remove the noise.

states and behaviors:
- The `portal` and `preventBodyScroll` props are set to `true`. They can
still be manually set to `false`.
- When the dialog is open, element tree outside it will be inert.
Copy link
Contributor Author

@ciampo ciampo Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mirka here's another instance of the problem you were mentioning with list being merged when rendered to HTML ( #68209 (comment) )

@@ -1,344 +1,589 @@
# `Menu`
# Menu
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mirka there's no way currently to mark the component as private in the README, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I'll figure something out as a follow up.

@ciampo ciampo self-assigned this Dec 23, 2024
@ciampo ciampo requested a review from a team December 23, 2024 16:26
@ciampo ciampo added [Type] Developer Documentation Documentation for developers [Package] Components /packages/components labels Dec 23, 2024
@ciampo ciampo marked this pull request as ready for review December 23, 2024 16:26
@ciampo ciampo requested a review from ajitbohra as a code owner December 23, 2024 16:26
Copy link

github-actions bot commented Dec 23, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ciampo <[email protected]>
Co-authored-by: mirka <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link

Flaky tests detected in 364338a.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12470206963
📝 Reported issues:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants